fix(spec,runtime): 服务槽查找返回它的契约而不是 any —— 开机几分钟就又抓出两处 (#4127) - #4168
Merged
os-zhuang merged 1 commit intoJul 30, 2026
Conversation
… not `any` (#4127) #4127's most valuable item was the one it did not do: add a gate for the class. Its four contract gaps were found by a human sweeping the dispatcher by hand. A sweep is not repeatable, and this one was not complete. The root was one line — `getService(name: string): any` in domain-handler-registry.ts. Against `any`, a domain calling a method its contract declares and a domain calling a method nobody declares typecheck identically. That is what let #4087 ship a `/storage` handler passing two arguments no implementation takes, and what hid #4127's four. `CoreServiceContracts` is the slot -> contract ledger. `CoreServiceName` named the slots and `contracts/*` described them; nothing connected the two. It does now, and `getService<K>(name: K)` resolves through it, so a call outside the contract is a compile error at the call site. An entry is a claim, so entries are made only where the binding is evidenced: by the provider that registers the slot (service-storage -> file-storage; objectql -> data, whose own comment reads "ObjectQL implements IDataEngine"), or by dispatcher work that proved it (#4143/#4150 for automation, notification, i18n). `ui` is deliberately unmapped — the slot exists and domains/ui.ts serves it, but no IUiService was ever written. An unmapped slot resolves to `unknown`, not `any`, so it must be cast deliberately and the gap stays legible. Two findings within minutes of turning it on: - `/auth` called a method that does not exist. domains/auth.ts probed `authService.handler(request, response)`; `IAuthService` declares `handleRequest(request)` and `AuthManager` implements exactly that, with no `handler`. False on every deployment — #4143's dead `automation.trigger` again. #4127's sweep never mentions `/auth` in either its gap list or its "clean" list: the file the compiler flagged first is the one the human pass skipped. Not a live hole — the Hono adapter calls `handleRequest` itself and only falls through when no usable auth service answered — but reading the contract makes the branch reachable for the first time, so a host calling `handleAuth` directly WITH an auth service now gets it instead of mockAuthFallback's `mock_<uuid>` session. - `POST /analytics/sql` invoked an optional method unguarded. `generateSql?` is optional on IAnalyticsService — unlike `query`/`getMeta` beside it — so a provider without it answered a 500 from TypeError instead of saying the capability is absent. Answers `handled: false` now, the same 404 the entry gate already gives for absent analytics capability. `isServiceServeable` becomes a type guard (`svc is NonNullable<T>`). Every domain already calls it first on a resolved slot, so one predicate narrows away the `undefined` for the whole body — the null check and the capability check were always the same check. The test-side hole #4127 predicted, closed for this batch: THREE tests across two files mocked `{ handler }` for auth, including one whose subject was the resolution path, so it proved the lookup worked and nothing about the call. `ContractMock<T>` guards mock keys against the contract; signatures stay `unknown` so vi.fn() does not force everything back to `as any`. The automation mock's `trigger` stays as a labelled negative control outside the checked literal — a test asserting the route never calls it is the point. The 12 domains not calling `getService` are untouched. `resolveService`, which also takes non-CoreServiceName names like `protocol` and `objectql`, is left for a later batch rather than widened here. Refs #4127 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 113 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 13:39
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…and the `: any` escapes on core slots are gone (#4127) (#4176) Batch 2 of the #4127 gate. #4168 typed `getService` — easy, because every one of its call sites already passed a `CoreServiceName`. `resolveService` is the mixed one, and it is where the remaining `any` lived. Overloads split it exactly where the evidence does. A `CoreServiceName` resolves to the slot's contract; anything else keeps `any`: - Core slots, however written. 17 call sites address a core slot with a bare literal (`'metadata'` x10, `'automation'` x3, `'auth'` x3, `'ai'`) rather than `CoreServiceName.enum.*` — the same slot addressed two ways. Both resolve to the contract now, with no edit to the call sites. - Everything else — `protocol` (x22), `objectql` (x9), `mcp`, `kernel-resolver`, `security`, `scope-manager`. Real services with no `CoreServiceName` entry and no written contract. They keep `any` rather than being given a shape here that nothing verifies: that `any` is where the ledger honestly ends, and writing those contracts is its own change. The typing was being erased at three call sites, and that is the actual finding. `const x: any = await deps.resolveService('auth', ...)` defeats all of this — the annotation wins and #4168's work does nothing there. Sweeping for the pattern found three on core slots: `/mcp` x2 — two more undeclared methods. The domain calls `authService?.getMcpResourceUrl?.()` and `?.getMcpResourceMetadataUrl?.()`. AuthManager implements both (plugin-auth uses them internally); IAuthService declared neither. Call site and implementation agree, the contract is the thing nobody wrote. The `: any` + optional-chaining combination made this WORSE than the earlier gaps, not better: invisible to the type system AND accidentally safe. An absent method returns `undefined`, so the skill route silently fell back to deriving an MCP URL from the request host — a real disagreement between the auth service's canonical value and the derived one would have looked like normal operation. The whole point of `getMcpResourceUrl` is that it comes off the auth `basePath` so the two CANNOT disagree about the API prefix; the route's own comment says "the auth service owns the canonical value". Both declared optional: an auth provider without MCP/OAuth support fills the slot legitimately, and `getMcpResourceMetadataUrl` returning `null` (OAuth track off) stays distinct from the method being absent. `/packages` x1 — `const metadata: any = await deps.getService(...metadata)`, feeding `new SeedLoaderService(ql, metadata, ...)`. Annotation dropped; it typechecks against IMetadataService now. Its neighbours `protocol` and `ql` keep their `any` for the honest reason above. No other core-slot lookup is annotated away — the sweep is exhaustive over domains/*.ts. `api-surface.json` is unchanged: the two additions are interface MEMBERS, not new exports. Refs #4127 Claude-Session: https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw Co-authored-by: Claude <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…Name` (#4127) (#4202) Batch 3 of the #4127 gate, after #4168 (`getService`) and #4176 (`resolveService`). Three slots — `security`, `shareLinks`, `objectql` — each had a written contract, a provider registering them, and call sites already inside the contract. The only missing link was that the slot name was not a `CoreServiceName` member, so nothing could connect them and all three sat behind `as any`. The ledger extends past the enum rather than the enum growing. The two answer different questions, and conflating them is what left these untyped: `CoreServiceName` answers "what happens at boot when this slot is empty?" — it sits beside `ServiceCriticality` and drives startup orchestration and discovery, so adding a member changes runtime behaviour and is effectively permanent. The ledger answers "what shape occupies this slot?" — pure type information. These three need only the second, so `ServiceSlotContracts extends CoreServiceContracts` adds them there and `resolveService` keys on `keyof ServiceSlotContracts`. Zero runtime effect. If one is later promoted to a genuine core service, its entry moves up and nothing else changes; a test asserts these keys are NOT enum members, so that migration cannot happen silently. Evidence before an entry, as always: `plugin-security` registers `security` and `ISecurityService`'s own doc names that registration; `plugin-sharing` registers `ShareLinkService`, which declares `implements IShareLinkService`; and `objectql` is an ALIAS of `data` — `packages/objectql`'s plugin registers the same instance under both names two lines apart, so one object was resolving as `IDataEngine` through one name and `any` through the other. `protocol` (22 call sites) and `mcp` have no written contract and stay unmapped. Turning it on found four things, all on the `/security` admin surface: 1. Request input reached the security service unvalidated. `?status=` was `String(query.status)` — any string — handed to a method whose contract declares exactly three values, and from there into the query's `where` clause. Not an injection (the `where` is structured, never interpolated), but `?status=garbage` matched no row and returned an empty list, which reads as "there are no suggestions" rather than "that is not a status". Now a 400. 2. A test pinned that bug as expected behaviour. The existing case asserted `status: 'open'` — not one of the three declared values — reached the service and returned 200. It proved the delegate carried A FILTER and nothing about that filter being a status. Same shape as batch 1's `auth.handler` mocks: coverage in appearance, a wrong contract in substance. 3. and 4. Two writes could not prove they had a caller. `confirmAudienceBindingSuggestion`/`dismissAudienceBindingSuggestion` declare `callerContext: SecurityContext` non-optionally — deliberately, since the read beside them declares it optional — and the domain passed a possibly-`undefined` execution context. This was NOT a live hole, and the distinction matters: with no execution context `shouldDenyAnonymous` already denied, because it sees no `userId`/`isSystem` and its allowlist arm needs a non-empty `path` this seam never passes, so it fell through to `return true`. What it never did was narrow `ec` itself — it only read `ec?.userId`. Checking `ec` directly is behaviour-preserving and makes the invariant legible to the compiler and the next reader. The `?status=` rejection is the one BEHAVIOUR CHANGE: an unknown status was a silent empty list and is now a 400 naming the accepted values. The accepted set is a `Record` keyed on the contract type, so adding a status to the contract leaves a key missing and renaming one leaves a key excess — either fails to compile, where a plain array would have drifted silently. Documented on the permission-sets page, where the endpoint is described. Refs #4127
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
… which found the project-membership gate not gating (#4127) (#4214) Batch 4 of the #4127 gate. #4168/#4176/#4202 made a slot lookup return the slot's contract. Nothing protected that: an `any` annotation on the RESULT switches the checking back off for that call site, silently, with no test failing and no visual difference from code that has it. Three such sites already existed and were found by grep — the same unrepeatable sweep this work replaced. The rule bans `: any` / `as any` on a `resolveService` / `getService` / `getRequestKernelService` result. Slots with no written contract (`protocol`, `mcp`, `kernel-resolver`, `scope-manager`) are exempted BY NAME, CENTRALLY, in eslint.config.mjs — not by inline disables, because `pnpm lint` runs `--no-inline-config` and ignores those on purpose. The effect is the one worth having: a deliberate gap is a reviewed line in one file, a careless one is a build failure, and they stop looking identical in the code. ITS FIRST RUN FOUND A LIVE FAIL-OPEN. `enforceProjectMembership` read the session as `authService?.api?.getSession?.(...)` with no `getApi()` fallback — the only one of the codebase's three `.api` readers without it. `plugin-auth` registers `AuthManager`, which has NO `.api` member at all. So the read yielded `undefined`, `userId` stayed unset, and the function returned at its "anonymous — upstream auth will decide" line BEFORE ever querying `sys_environment_member`. A signed-in non-member passed the gate, on every deployment with project scoping on — which is where the flag defaults to true. Anonymous callers were still denied elsewhere (#2567/#3963), so this was specifically the signed-in non-member case. The existing test for that gate mocked auth as `{ api: { getSession } }` — the legacy shape the shipped provider does not have — so it was green throughout. That is the FOURTH test in this work line found encoding a contract nobody implements, after batch 1's three `auth.handler` mocks and batch 3's `status: 'open'`. The new test uses the `getApi()` shape and fails against the pre-fix code. Also found by the rule, all the same #4127 shape (implemented, called, undeclared) and all now declared: IAuthService gains `api`, `getApi`, `isAuthGateActive` and `verifyMcpAccessToken`; IMetadataService gains `load` and `loadDiagnosed`. `getApi`'s return type is the EVIDENCED SUBSET — `getSession({headers})` and the three fields callers read — not a re-declaration of better-auth's handle, which belongs to that library. And the pattern's real root: the lookup facade returning `any` was re-declared in THREE places. Batches 1-3 typed `DomainHandlerDeps` and left `ActionExecutionDeps` and resolve-execution-context's `ResolveOptions` still saying `any` — so the copy that stayed untyped was the way around all the others, and it is where the auth reads lived. All three are typed now. Completing the interface: `getRequestKernelService` gets the same overload split (its one caller resolves the same `objectql` slot the `resolveService` fallback beside it does, so the two arms of one expression had different types), and share-links' `getEngine` loses a `Promise<any>` return annotation — a THIRD erasure syntax after `: any` and `as any`, and one this AST rule cannot see. That residual is documented in the config. `getObjectQL` STAYS `any`, deliberately, with the reason recorded: it exists to reach ObjectQL's surface beyond IDataEngine (`registry`, `executeAction`), which has no contract. Typing it IDataEngine would be the comfortable-looking lie. The auth and metadata contract doc pages are brought back in step with the interfaces — the auth page still called itself "intentionally minimal" while missing six members, two of them from batch 2. Refs #4127
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4127。这条做的是那个 issue 里最值钱、但它自己没做的一项:「外加一条更值钱的:给这个类别加个 gate」。
#4127 编目的四处契约缺口,是靠人拿尺子把 dispatcher 扫了一遍找出来的。扫描不可重复,而且这一次也并不完整 —— 见下面的
/auth。根在一行
面对
any,「domain 调用契约声明的方法」和「domain 调用根本没人声明的方法」类型检查完全一样。这就是 #4087 能带着一个「传了两个没有任何实现接受的参数」的/storagehandler 上线几个月的原因,也是 #4127 那四处藏身之处。1.
CoreServiceContracts—— 槽 → 契约的账本CoreServiceName命名了槽,contracts/*描述了槽,但没有任何东西把两者连起来。现在连上了,getService<K>(name: K)经由它解析,于是越出契约的调用是调用点上的编译错误。一条 entry 就是一个断言,所以只在有证据时才写:要么来自注册该槽的提供方(
service-storage→file-storage;objectql→data,它自己的注释就写着 "ObjectQL implements IDataEngine"),要么来自已经证明过对应关系的 dispatcher 工作(#4143 / #4150 的automation、notification、i18n)。ui故意留空:槽存在、domains/ui.ts在服务它,但IUiService从来没被写出来过。未映射的槽解析为unknown而不是any—— 必须显式转换,于是缺口保持可见,而不是看起来像已检查。2. 开机后几分钟,两处真发现
①
/auth调用了一个不存在的方法domains/auth.ts探测的是authService.handler(request, response)。而IAuthService声明的是handleRequest(request): Promise<Response>,AuthManager实现的正是后者,根本没有handler。所以这个探测在任何部署上都是 false —— #4143 里那个死掉的automation.trigger又一次。不是线上漏洞:Hono adapter 自己调
handleRequest(packages/adapters/hono/src/index.ts:362),只有在没有可用 auth 服务时才落到 dispatcher,所以那条部署链路上没有东西被 mock 静默服务过。但改读契约让这个分支第一次变得可达 —— 一个直接调handleAuth且注册了 auth 服务的 host,以前拿到的是mockAuthFallback的mock_<uuid>会话,现在拿到的是真的 auth 服务。②
POST /analytics/sql无守卫地调用可选方法generateSql?在IAnalyticsService上是可选的 —— 和它旁边的query/getMeta不同 —— 而调用点没有探测。于是一个没实现它的提供方会因TypeError返回 500,而不是说「这项能力不存在」。service-analytics 实现了它,所以一直没人注意;但契约允许没有它的提供方,而这个槽按设计就是多提供方的。现在返回handled: false,也就是本文件入口守卫对「analytics 能力缺席」已经给出的那个 404。3.
isServiceServeable变成类型守卫svc is NonNullable<T>。每个 domain 本来就把它作为拿到槽后的第一件事,所以这一个谓词就为整个 handler body 收窄掉undefined—— 空值检查和能力检查本来就是同一个检查。4. #4127 预言的测试侧洞,本批次关闭
那个 issue 的最后一节说得很准:mock 写的是 handler 想要的形状,于是 handler 和它的测试互相印证,而与任何实现都不一致。
auth 这一处,三个测试、跨两个文件都 mock 了
{ handler }—— 其中一个的主题根本是解析路径(getServiceAsync优先于getService),所以它证明了查找有效,却对调用本身一无所证。ContractMock<T>(Partial<Record<keyof T, unknown>>)现在守着 mock:键对着契约校验,签名故意留unknown—— 强行匹配签名只会把所有 mock 推回as any,那正是这次要修的状态。automation mock 里的trigger确实不在契约上,它作为显式标注的反向对照留在受检字面量之外:一个断言「这条路由绝不调用它」的测试,正是它存在的意义。范围
没有重命名,除上述两处修复外无运行时行为变化。12 个不调用
getService的 domain 未被触及。resolveService(它还接受protocol、objectql这类非CoreServiceName的名字)留给下一批,没有在这里顺手放宽。验证
@objectstack/runtime@objectstack/spectsc --noEmitpnpm lintcheck:*门禁check:api-surface,0 breaking / 2 added,快照已重生成)🤖 Generated with Claude Code
https://claude.ai/code/session_015T5CeitwV1jXLvsL1A84tw
Generated by Claude Code